Skip to content

[Custom Transactions] Add TxBuilder trait, support fixed additional outputs #3775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[Custom Transactions] Add TxBuilder trait, support fixed additional outputs #3775

wants to merge 4 commits into from

Conversation

tankyleo
Copy link
Contributor

@tankyleo tankyleo commented May 13, 2025 •
edited
Loading

hi @TheBlueMatt on this branch here i make an attempt at an API that supports "fixed additional outputs" on commitment transactions.

I place today's 330 sat anchors in that bucket, but the API should allow for custom ones.

The API should also let people customize all the existing outputs on the commitment transaction.

Still need to think about any remaining TODOs for additional outputs that are only present for some commitments (eg. DLC outputs), depending on how we'd like to prioritize these.

One TODO that remains is abstracting away the calls to chan_utils::commit_and_htlc_tx_fees_sat in channel dust exposure calculations.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 13, 2025 •
edited
Loading

👋 Thanks for assigning @carlaKC as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tankyleo tankyleo force-pushed the fixed-addl-outputs branch from c245ca1 to 7fc2092 Compare May 13, 2025 04:58
@tankyleo tankyleo force-pushed the fixed-addl-outputs branch from 7fc2092 to 6aa0848 Compare May 21, 2025 04:43
@carlaKC carlaKC mentioned this pull request May 21, 2025
Open
38 tasks
@tankyleo tankyleo mentioned this pull request Apr 10, 2025
Open
21 tasks
@tankyleo tankyleo force-pushed the fixed-addl-outputs branch from fa3ea56 to 14b83a3 Compare May 22, 2025 05:20
@codecov Codecov
Copy link

codecov bot commented May 22, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.10309% with 19 lines in your changes missing coverage. Please review.

Project coverage is 90.37%. Comparing base (5316257) to head (14b83a3).
Report is 49 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 92.40% 5 Missing and 14 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3775      +/-   ##
==========================================
+ Coverage   89.53%   90.37%   +0.84%     
==========================================
  Files         157      160       +3     
  Lines      125310   133578    +8268     
  Branches   125310   133578    +8268     
==========================================
+ Hits       112192   120721    +8529     
+ Misses      10429    10231     -198     
+ Partials     2689     2626      -63

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tankyleo tankyleo requested a review from TheBlueMatt May 22, 2025 05:30
@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 2nd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 3rd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@carlaKC carlaKC self-requested a review May 29, 2025 17:12
carlaKC
carlaKC reviewed May 30, 2025
View reviewed changes
carlaKC
carlaKC reviewed May 30, 2025
View reviewed changes
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a reasonable API on first pass! Will have something smarter to say once I've rebased on this 👌

@ldk-reviews-bot
Copy link

🔔 4th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 5th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 6th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 7th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 8th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 9th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlaKC carlaKC carlaKC left review comments

@TheBlueMatt TheBlueMatt Awaiting requested review from TheBlueMatt

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tankyleo @ldk-reviews-bot @carlaKC